Skip to content

Conversation

@captainzmc
Copy link
Member

What changes were proposed in this pull request?

see: https://issues.apache.org/jira/browse/RATIS-1547

Comment on lines +51 to +52
public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@captainzmc , sorry that GrpcClientProtocolService is not a public API. We should not change it for an Ozone unit test. We should fix the test instead.

class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
private static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
public class GrpcClientProtocolService extends RaftClientProtocolServiceImplBase {
public static final Logger LOG = LoggerFactory.getLogger(GrpcClientProtocolService.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also encountered compile error while trying recent Ratis changes (RATIS-1515, etc.) with Ozone. But I think exposing LOG as public only for Ozone test is bad practice. We may be able to improve TestRatisPipelineLeader by using custom retry policy.

@captainzmc
Copy link
Member Author

Thanks @szetszwo and @adoroszlai for the suggestion. Agree with you, let’s improve TestRatisPipelineLeader to avoid this error.

@captainzmc captainzmc closed this Mar 3, 2022
@szetszwo
Copy link
Contributor

@captainzmc , filed https://issues.apache.org/jira/browse/HDDS-6461 for updating Ratis snapshot version including fixing TestRatisPipelineLeader; see apache/ozone#3205

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants